Personal copy of anomalyco/opencode#29208: fix(config): catch parse errors gracefully during startup#4
Personal copy of anomalyco/opencode#29208: fix(config): catch parse errors gracefully during startup#4HaleTom wants to merge 32 commits into
Conversation
Wrap ConfigParse.jsonc() and ConfigParse.effectSchema() calls in
loadConfig with Effect.catchCause to prevent sync-thrown JsonError
and InvalidError from becoming Effect defects. On parse/schema
failure, the bad file is skipped with a log.error and {} fallback
instead of crashing, matching the existing tui.jsonc error handling
pattern in tui.ts.
Without this, any invalid JSONC syntax or schema violation in
opencode.json/opencode.jsonc causes "4 of 6 requests failed:
Unexpected server error" on startup. Now the server starts with
default values and logs the config error details.
Fixes anomalyco#29200
|
Review changes with Changed Files
|
|
Hey! Your PR title Please update it to start with one of:
Where See CONTRIBUTING.md for details. |
|
This PR doesn't fully meet our contributing guidelines and PR template. What needs to be fixed:
Please edit this PR description to address the above within 2 hours, or it will be automatically closed. If you believe this was flagged incorrectly, please let a maintainer know. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a significant architectural shift and UI overhaul across the OpenCode platform. Key updates include migrating the desktop application from Tauri to Electron, implementing a new tab-based navigation and composer design for non-production environments, and adding a referral reward system to the console. The codebase underwent a major refactoring to rename global SDK and sync providers to server-specific contexts, alongside infrastructure updates to monitoring and deployment regions. Review feedback highlights a critical accessibility concern regarding hidden streaming content, warns against breaking changes in custom provider configurations due to consolidated placeholders, and recommends using reactive state over direct DOM access in the prompt input component.
|
@gemini-code-assist please |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
packages/opencode/src/config/config.ts:446
- When config parsing/schema decoding fails,
datafalls back to an empty decodedInfo, but the function still proceeds to the$schemaauto-injection block. This can overwrite an invalid config file (or duplicate an existing$schemakey) even though the file couldn't be parsed/validated, which is potentially destructive and can further corrupt the user's config.
Consider short-circuiting on parse/validation failure (e.g., return early for path sources) or tracking a parsedSuccessfully flag so plugin resolution and $schema auto-write only run when parsing/validation succeeded.
const data = yield* Effect.sync(() => {
const parsed = ConfigParse.jsonc(expanded, source)
return ConfigParse.schema(Info, normalizeLoadedConfig(parsed, source), source)
}).pipe(
Effect.catchCause((cause) =>
Effect.sync(() => {
log.error("invalid config", { path: source, cause })
return Schema.decodeSync(Info)({})
}),
),
)
if (!("path" in options)) return data
yield* Effect.promise(() => resolveLoadedPlugins(data, options.path)).pipe(
Effect.catchCause((cause) =>
Effect.sync(() => {
log.error("plugin resolution failed", { path: source, cause })
}),
),
)
if (!data.$schema) {
data.$schema = "https://opencode.ai/config.json"
const updated = text.replace(/^\s*\{/, '{\n "$schema": "https://opencode.ai/config.json",')
yield* fs.writeFileString(options.path, updated).pipe(Effect.catch(() => Effect.void))
|
The implementation for graceful error handling using One minor suggestion: In Severity: |
|
The implementation of the |
1 similar comment
|
The implementation of the |
|
Addressed the log sanitization suggestion: replaced |
- Use safe fallback for Info decode ({} as Info) instead of Schema.decodeSync
- Use Cause.pretty for plugin resolution error logging
- Strengthen test assertion with specific value
|
Review 4358274911 is a duplicate of review 4358240107 — both comments were already addressed and resolved:
No new actionable items here. |
- Strip unknown config keys before schema validation to preserve valid fields - Sanitize log.error calls to avoid leaking config content via cause - Handle plugin resolution per-item to avoid partial mutation on failure - Add test for plugin resolution failure
|
This pull request has been automatically closed because it was not updated to meet our contributing guidelines within the 2-hour window. Feel free to open a new pull request that follows our guidelines. |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
| function stripUnknownKeys(data: unknown): unknown { | ||
| if (typeof data !== "object" || data === null || Array.isArray(data)) return data | ||
| const known = new Set(Info.ast.propertySignatures.map((p) => String(p.name))) | ||
| const result: Record<string, unknown> = {} | ||
| for (const [key, value] of Object.entries(data)) { | ||
| if (known.has(key)) result[key] = value | ||
| else log.warn("config key is not recognized and will be ignored", { key }) |
| Effect.catchCause((cause) => | ||
| Effect.sync(() => { | ||
| const errors = cause.reasons | ||
| .filter(Cause.isDieReason) | ||
| .map((r) => (r.defect instanceof Error ? r.defect.name : "UnknownError")) | ||
| .join(", ") | ||
| log.error("invalid config: config file could not be parsed", { path: source, error: errors }) | ||
| return {} as Info | ||
| }), | ||
| ), |
| it.instance("handles plugin resolution failure gracefully", () => | ||
| Effect.gen(function* () { | ||
| const test = yield* TestInstance | ||
| yield* writeConfigEffect(test.directory, { | ||
| $schema: "https://opencode.ai/config.json", | ||
| model: "has-plugin", | ||
| plugin: ["./non-existent-plugin.ts"], | ||
| }) | ||
| const config = yield* Config.use.get() | ||
| expect(config.model).toBe("has-plugin") |
Personal copy of anomalyco/opencode#29208